-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract all yarn related methods to azkaban-common as shared util library #3145
Extract all yarn related methods to azkaban-common as shared util library #3145
Conversation
final String logFilePath = jobProps.getString(CommonJobProperties.JOB_LOG_FILE); | ||
logger.info("Log file path is: " + logFilePath); | ||
|
||
HadoopJobUtils.proxyUserKillAllSpawnedHadoopJobs(logFilePath, jobProps, tokenFile, logger); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not good: duplicated info in the 2 parameters
@@ -0,0 +1,71 @@ | |||
package azkaban.utils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically moved from old HadoopJobUtils, but follow the idea of yarnClient can be passed in
74f379a
to
9716e64
Compare
public static YarnClient createYarnClient(Props props) { | ||
final YarnConfiguration yarnConf = new YarnConfiguration(); | ||
final YarnClient yarnClient = YarnClient.createYarnClient(); | ||
if (props.containsKey(YARN_CONF_DIRECTORY_PROPERTY)) { | ||
yarnConf.addResource( | ||
new Path(props.get(YARN_CONF_DIRECTORY_PROPERTY) + "/" + YARN_CONF_FILENAME)); | ||
} | ||
yarnClient.init(yarnConf); | ||
yarnClient.start(); | ||
return yarnClient; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate yarnclient creation for later reuse
Codecov Report
@@ Coverage Diff @@
## master #3145 +/- ##
============================================
+ Coverage 41.73% 41.87% +0.14%
- Complexity 4712 4740 +28
============================================
Files 600 606 +6
Lines 40397 40564 +167
Branches 4703 4709 +6
============================================
+ Hits 16860 16988 +128
- Misses 22134 22172 +38
- Partials 1403 1404 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
az-hadoop-jobtype-plugin/src/main/java/azkaban/jobtype/HadoopJobUtils.java
Show resolved
Hide resolved
9716e64
to
a35ff0e
Compare
looks good to me now, ship it. |
ca8481a
to
a35ff0e
Compare
a35ff0e
to
0a62250
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on refactor and adding test. The test file is missing the copy right header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…util library (azkaban#3145)" This reverts commit a31c30d.
set HADOOP_TOKEN_FILE_LOCATION to token file add logging around tokens remove the token-file deletion code Extract all yarn related methods to azkaban-common as shared util library (azkaban#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file
…il library (#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file
…rary (#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file RB=3636861 G=azkabandev-reviewers R=anath1,ypadron,angoel,adsharma,jakhani,areznik,gsalia,apruthi,sarumuga,prkhande,abtiwari,djaiswal,sshardoo A=bijiang
…util library (#3145)" (#3154) This reverts commit a31c30d. Reason: this change doesn't work with containerization, will fix the issue and raise a new PR for this. 3637010 RB=3637010 G=azkabandev-reviewers R=ypadron,jakhani,prkhande,sarumuga,angoel,adsharma,abtiwari,gsalia,djaiswal,apruthi,anath1,sshardoo,areznik A=bijiang
Issue to solve: A part of the fixing / upgrade of the yarn application killing logic: make it easier to add and reuse "call yarn cluster for application ids" method later on
Changes made: Some simple refactor of the code, extract all the yarn-client related methods to an azkaban-common util s class
Testing done: